Skip to content

Add Default impls for Pinned Box, Rc, Arc #143717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 16, 2025

Conversation

Jules-Bertholet
Copy link
Contributor

@Jules-Bertholet Jules-Bertholet commented Jul 10, 2025

Fixes #143688.

@rustbot label T-libs-api needs-fcp

Also needs a crater run, as the Box impls could theoretically be breaking due to #[fundamental] (though a cursory search suggests this is unlikely to cause issues).

@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 10, 2025
@tgross35 tgross35 added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jul 10, 2025
@tgross35
Copy link
Contributor

@bors2 try

@rust-bors
Copy link

rust-bors bot commented Jul 10, 2025

⌛ Trying commit 9060b6b with merge 5ef65a4

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jul 10, 2025
Add `Default` impls for `Pin`ned `Box`, `Rc`, `Arc`

Fixes #143688.

`@rustbot` label T-libs-api needs-fcp

Also needs a crater run, as the `Box` impls could theoretically be breaking due to `#[fundamental]` (though a [cursory search](https://github.com/search?q=%2Fimpl%28%3C.*%3E%29%3F+Default+for+Pin%3C%2F+path%3A*.rs&type=code) suggests this is unlikely to cause issues).
@rust-bors
Copy link

rust-bors bot commented Jul 10, 2025

☀️ Try build successful (CI)
Build commit: 5ef65a4 (5ef65a4ed75279dc1b7a26cedc27293a9f4ca8db, parent: 32cd9114712a24010b0583624dc52ac302194128)

@tgross35
Copy link
Contributor

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-143717 created and queued.
🤖 Automatically detected try build 5ef65a4
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2025
@craterbot
Copy link
Collaborator

🚧 Experiment pr-143717 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-143717 is completed!
📊 7 regressed and 5 fixed (662328 total)
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jul 12, 2025
@Jules-Bertholet
Copy link
Contributor Author

All spurious failures. “prepare-fail (112811)” seems not-great though. Looks like a lot of “no space left on device” errors? If an issue with crater is preventing a large proportion of the crates from being properly tested…

@Skgland
Copy link
Contributor

Skgland commented Jul 12, 2025

All spurious failures. “prepare-fail (112811)” seems not-great though. Looks like a lot of “no space left on device” errors? If an issue with crater is preventing a large proportion of the crates from being properly tested…

Note that the prepare-fail category is a relatively recent change, they used to be categorized as error. Also prepare-fail results are included in the retry-regressed-list.txt

@Amanieu Amanieu removed T-libs Relevant to the library team, which will review and decide on the PR/issue. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Jul 15, 2025
@Amanieu
Copy link
Member

Amanieu commented Jul 15, 2025

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Jul 15, 2025

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 15, 2025
@Amanieu Amanieu removed the needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. label Jul 15, 2025
@dtolnay dtolnay added needs-crater This change needs a crater run to check for possible breakage in the ecosystem. and removed needs-crater This change needs a crater run to check for possible breakage in the ecosystem. labels Jul 15, 2025
@dtolnay
Copy link
Member

dtolnay commented Jul 15, 2025

Consider generalizing the Pin<Box<>> impl like this:

  impl<T> Default for Pin<Box<T>>
  where
-     T: Default,
+     T: ?Sized,
+     Box<T>: Default,

This would apply to types like Pin<Box<serde_json::value::RawValue>>, where RawValue does not implement Default but Box<RawValue> does.

@rfcbot concern Box<T>: Default

@Jules-Bertholet
Copy link
Contributor Author

@dtolnay I’ve done as you asked.

@dtolnay
Copy link
Member

dtolnay commented Jul 15, 2025

@rfcbot resolve Box<T>: Default

@tgross35
Copy link
Contributor

For after fcp,
r? @dtolnay

@rustbot rustbot assigned dtolnay and unassigned tgross35 Jul 29, 2025
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 5, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Aug 5, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Aug 15, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Aug 15, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Aug 15, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 15, 2025

📌 Commit 87dd469 has been approved by dtolnay

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2025
bors added a commit that referenced this pull request Aug 15, 2025
Rollup of 11 pull requests

Successful merges:

 - #143717 (Add `Default` impls for `Pin`ned `Box`, `Rc`, `Arc`)
 - #144054 (Stabilize as_array_of_cells)
 - #144907 (fix: Reject async assoc fns of const traits/impls in ast_passes)
 - #144922 (Implement `#[derive(From)]`)
 - #144963 (Stabilize `core::iter::chain`)
 - #145436 (fix(tests/rmake/wasm-unexpected-features): change features from `WASM1` to `MVP`)
 - #145453 (Remove duplicated tracing span in bootstrap)
 - #145454 (Fix tracing debug representation of steps without arguments in bootstrap)
 - #145455 (Do not copy files in `copy_src_dirs` in dry run)
 - #145462 (Stabilize `const_exposed_provenance` feature)
 - #145466 (Enable new `[range-diff]` feature in triagebot)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 41e40f3 into rust-lang:master Aug 16, 2025
11 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 16, 2025
rust-timer added a commit that referenced this pull request Aug 16, 2025
Rollup merge of #143717 - Jules-Bertholet:pin-default, r=dtolnay

Add `Default` impls for `Pin`ned `Box`, `Rc`, `Arc`

Fixes #143688.

`@rustbot` label T-libs-api needs-fcp

Also needs a crater run, as the `Box` impls could theoretically be breaking due to `#[fundamental]` (though a [cursory search](https://github.com/search?q=%2Fimpl%28%3C.*%3E%29%3F+Default+for+Pin%3C%2F+path%3A*.rs&type=code) suggests this is unlikely to cause issues).
@jhpratt
Copy link
Member

jhpratt commented Aug 16, 2025

still in queue; merged in #145475

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 16, 2025
@Jules-Bertholet Jules-Bertholet deleted the pin-default branch August 16, 2025 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pin<Box<impl Default>> should impl Default
10 participants